Feature(cli): Add --sync to prevent AI agents from breaking the host files#4429
Feature(cli): Add --sync to prevent AI agents from breaking the host files#4429jandubois merged 1 commit intolima-vm:masterfrom
--sync to prevent AI agents from breaking the host files#4429Conversation
cmd/limactl/shell.go
Outdated
| shellCmd.Flags().Bool("reconnect", false, "Reconnect to the SSH session") | ||
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().Bool("sync-host-workdir", false, "Copy the host working directory to the guest to run AI commands inside VMs (prevents AI agents from breaking the host files)") |
There was a problem hiding this comment.
- For consistency with
--mount ., probably this should be like--sync . - The flag is not really specific to AI commands. Use case with AI should be mentioned in https://lima-vm.io/docs/examples/ai/ though.
cmd/limactl/shell.go
Outdated
|
|
||
| const ( | ||
| rsyncMinimumSrcDirDepth = 4 // Depth of "/Users/USER" is 3. | ||
| guestSyncedWorkdir = "~/synced-workdir" |
There was a problem hiding this comment.
Is there any reason that we cannot just use the same path as the host dir?
Is that for avoiding conflicts with mounts?
There was a problem hiding this comment.
Is there any reason that we cannot just use the same path as the host dir?
Yes, issue with rsync because it only tries to create the base of the path and not the full path so using for example ansumansahoo@127.0.0.1:~/Users/ansumansahoo/Documents/GOLANG/lima as a destination path will result into an error rsync: [Receiver] mkdir "/home/ansumansahoo.linux/Users/ansumansahoo/Documents/GOLANG/lima" failed: No such file or directory (2)
Or do you mean to use only ansumansahoo@127.0.0.1:~/lima regarding this context?
There was a problem hiding this comment.
it only tries to create the base of the path and not the full path
Why not run mkdir -p
| } | ||
|
|
||
| for { | ||
| ans, err := uiutil.Select(message, options) |
There was a problem hiding this comment.
rsync can be slow and may eat up the host disk space, so this prompt should be shown before running rsync to the tmp dir
cmd/limactl/shell.go
Outdated
| return | ||
| case 2: // View the changed contents | ||
| diffCmd := exec.CommandContext(ctx, "diff", "-ru", "--color=always", hostCurrentDir, filepath.Join(hostTmpDest, filepath.Base(hostCurrentDir))) | ||
| lessCmd := exec.CommandContext(ctx, "less", "-R") |
cmd/limactl/shell.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("failed to get sync-host-workdir flag: %w", err) | ||
| } else if syncHostWorkdir && len(inst.Config.Mounts) > 0 { | ||
| return errors.New("cannot use `--sync-host-workdir` when the instance has host mounts configured, use `--mount-none` to disable mounts") |
There was a problem hiding this comment.
| return errors.New("cannot use `--sync-host-workdir` when the instance has host mounts configured, use `--mount-none` to disable mounts") | |
| return errors.New("cannot use `--sync-host-workdir` when the instance has host mounts configured, start the instance with `--mount-none` to disable mounts") |
cmd/limactl/shell.go
Outdated
| } | ||
| } else { | ||
| case syncHostWorkdir: | ||
| changeDirCmd = fmt.Sprintf("cd %s/%s", guestSyncedWorkdir, filepath.Base(hostCurrentDir)) |
There was a problem hiding this comment.
Make sure to quote the path, as it may contain spaces and quote symbols
There was a problem hiding this comment.
(Such paths should be tested in bats)
cmd/limactl/shell.go
Outdated
| } | ||
| rsyncCmd := exec.CommandContext(ctx, "rsync", rsyncArgs...) | ||
| rsyncCmd.Stdin = os.Stdin | ||
| rsyncCmd.Stdout = os.Stdout |
There was a problem hiding this comment.
cmd/limactl/shell.go
Outdated
| destination, | ||
| } | ||
| rsyncCmd := exec.CommandContext(ctx, "rsync", rsyncArgs...) | ||
| rsyncCmd.Stdin = os.Stdin |
There was a problem hiding this comment.
NVM, it was a mistake!
|
Left with adding bats test and docs. |
|
How can I test existing bats test files? ➜ lima2 git:(feat/rsync) ✗ ./hack/bats/lib/bats-core/bin/bats ./hack/bats/tests/list.bats
0 tests, 0 failures
➜ lima2 git:(feat/rsync) ✗ ./hack/bats/lib/bats-core/bin/bats --version
Bats 1.12.0
➜ lima2 git:(feat/rsync) ✗ lm -v
limactl version 2.0.1-122-g997e1287.m[EDIT]: Ran a sample bats test and it works: #!/usr/bin/env bats
@test "simple test" {
run echo "hello"
[ "$status" -eq 0 ]
[ "$output" = "hello" ]
}➜ lima2 git:(feat/rsync) ✗ ./hack/bats/lib/bats-core/bin/bats test-simple.bats
test-simple.bats
✓ simple test
1 test, 0 failures |
sync-host-workdir to prevent AI agents from breaking the host files--sync to prevent AI agents from breaking the host files
This was due to the fact that my Bash version which comes out of the box on MacOS was 3.2 which was silently failing because Bash 3.2 does not support associative arrays(introduced in 4.0). Had to upgrade it using brew and now it works! lima/hack/bats/helpers/load.bash Line 81 in d8828f4 |
cmd/limactl/shell.go
Outdated
| if pager == "" { | ||
| pager = "less" | ||
| } | ||
| lessCmd := exec.CommandContext(ctx, pager, "-R") |
There was a problem hiding this comment.
Not every PAGER supports this flag
| # Syncing Working Directory | ||
|
|
||
| The `--sync` flag for `limactl shell` enables bidirectional synchronization of your host working directory with the guest VM. This is particularly useful when running AI agents (like Claude, Copilot, or Gemini) inside VMs to prevent them from accidentally modifying or breaking files on your host system. | ||
|
|
There was a problem hiding this comment.
Probably needs a table to compare mount vs sync
96e3dd1 to
d501990
Compare
cmd/limactl/shell.go
Outdated
| return fmt.Errorf("failed to create the synced workdir in guest instance: %w", err) | ||
| } | ||
|
|
||
| if runtime.GOOS == "darwin" { |
There was a problem hiding this comment.
MacOS’s version of rsync (the latest being 2.6.9) doesn’t handle shell escaping itself. However, Linux versions of rsync (specifically 3.x) efficiently manage this. A shell-escaped path in a Linux environment caused path not found issues.
See this
There was a problem hiding this comment.
Better to add a comment
There was a problem hiding this comment.
Shouldn’t it check the version, not OS?
| |----------------|-------------| | ||
|
|
||
| The `--sync` flag for `limactl shell` enables bidirectional synchronization of your host working directory with the guest VM. This is particularly useful when running AI agents (like Claude, Copilot, or Gemini) inside VMs to prevent them from accidentally modifying or breaking files on your host system. | ||
|
|
There was a problem hiding this comment.
The comparison table should be shown before the example commands
| 1. Create an isolated instance for AI agents which must be started without host mounts for `--sync` to work: | ||
|
|
||
| ```bash | ||
| limactl start --name=ai-sandbox --mount-none template://default |
cmd/limactl/shell.go
Outdated
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().String("sync", "", "Copy the host working directory to the guest and vice-versa upon exit") | ||
| shellCmd.Flags().Lookup("sync").NoOptDefVal = "." |
There was a problem hiding this comment.
Not sure if we want NoOptDefVal.
Probably this one should be omitted from this PR, and should be revisited in a separate issue.
cmd/limactl/shell.go
Outdated
| shellCmd.Flags().Bool("reconnect", false, "Reconnect to the SSH session") | ||
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().String("sync", "", "Copy the host working directory to the guest and vice-versa upon exit") |
There was a problem hiding this comment.
| shellCmd.Flags().String("sync", "", "Copy the host working directory to the guest and vice-versa upon exit") | |
| shellCmd.Flags().String("sync", "", "Copy a host directory to the guest and vice-versa upon exit") |
| See also <https://github.com/anomalyco/opencode>. | ||
| {{% /tab %}} | ||
| ```bash | ||
| limactl shell --sync default claude "Add error handling to all functions" |
There was a problem hiding this comment.
| limactl shell --sync default claude "Add error handling to all functions" | |
| limactl shell --sync . default claude "Add error handling to all functions" |
| {{< /tabpane >}} | ||
| Or simply shell into the instance and make changes: | ||
| ```bash | ||
| limactl shell --sync default |
cmd/limactl/shell.go
Outdated
| // := "cd hostCurrentDir || cd hostHomeDir" if workDir == "" | ||
| var changeDirCmd string | ||
| var hostCurrentDir string | ||
| if syncDirVal != "" && syncDirVal != "." { |
There was a problem hiding this comment.
Why need to check syncDirVal != "." here?
There was a problem hiding this comment.
Oh, I didn't know that filepath.Abs() translates . to cwd.
jandubois
left a comment
There was a problem hiding this comment.
Mostly looking good, but the hard-coded home directory name should be fixed.
cmd/limactl/shell.go
Outdated
| } | ||
| } else { | ||
| case syncHostWorkdir: | ||
| destRsyncDir = fmt.Sprintf("/home/%s.linux%s", *inst.Config.User.Name, hostCurrentDir) |
There was a problem hiding this comment.
The home directory is configurable; don't recompute the default value:
| destRsyncDir = fmt.Sprintf("/home/%s.linux%s", *inst.Config.User.Name, hostCurrentDir) | |
| destRsyncDir = *inst.Config.User.Home + hostCurrentDir |
There is another instance of this further down in the file.
cmd/limactl/shell.go
Outdated
| remoteSource := fmt.Sprintf("%s:%s", *inst.Config.User.Name+"@"+inst.SSHAddress, destRsyncDir) | ||
| clean := filepath.Clean(hostCurrentDir) | ||
| parts := strings.Split(clean, string(filepath.Separator)) | ||
| dirForCleanup := shellescape.Quote(fmt.Sprintf("/home/%s.linux/", *inst.Config.User.Name) + parts[1]) |
There was a problem hiding this comment.
Use User.Home instead. Also check that strings.Split() has returned at least 2 elements.
| if err := rsyncDirectory(ctx, cmd, sshCmd, remoteSource, filepath.Dir(hostCurrentDir)); err != nil { | ||
| logrus.WithError(err).Warn("Failed to sync back the changes to host") | ||
| return | ||
| } |
There was a problem hiding this comment.
Should this function return an error? Why are these failures just logging a warning, and then continue as if everything is ok?
cmd/limactl/shell.go
Outdated
| ans, err := uiutil.Select(message, options) | ||
| if err != nil { | ||
| if errors.Is(err, uiutil.InterruptErr) { | ||
| logrus.Fatal("Interrupted by user") |
There was a problem hiding this comment.
Fatal exits the process right away and should be reserved for unrecoverable errors. This feels more like a shortcut for regular control flow because we can't return an error.
| source, | ||
| destination, | ||
| } | ||
| rsyncCmd := exec.CommandContext(ctx, "rsync", rsyncArgs...) |
There was a problem hiding this comment.
Please create an issue for it if you don't include it in the PR.
|
@jandubois Can we merge this and release v2.1 alpha ? |
Sorry, I didn't realize this. I thought 2.1 was planned for KubeCon. I can't review until tomorrow, but I'm fine with you merging it if you and @unsuman both think it is fine. It will just be an alpha, after all. |
cmd/limactl/shell.go
Outdated
| if pager == "" { | ||
| pager = "less" | ||
| } | ||
| lessCmd := exec.CommandContext(ctx, pager) |
There was a problem hiding this comment.
This doesn't work when PAGER contains whitespaces
Yes, the plan is unchanged. |
|
CI failing |
Because the PR needs to be rebased on latest |
| var sshExecForRsync *exec.Cmd | ||
| if syncHostWorkdir { | ||
| logrus.Infof("Syncing host current directory(%s) to guest instance...", hostCurrentDir) | ||
| sshExecForRsync = exec.CommandContext(ctx, sshExe.Exe, sshArgs...) |
There was a problem hiding this comment.
I think this is missing the port and host arguments. It looks like it works if it connects via the ControlMaster socket, but if that is broken, it will not create its own connection:
❯ rm ~/.lima/default/ssh.sock
❯ limactl shell --sync . --yes default pwd
INFO[0000] Syncing host current directory(/Users/jan/suse/lima/hack/bats) to guest instance...
FATA[0000] failed to create the synced workdir in guest instance: exit status 255There was a problem hiding this comment.
I think the BATS test need to include variants that delete the ssh.sock to properly test the explicit SSH commands.
cmd/limactl/shell.go
Outdated
| } | ||
|
|
||
| func executeSSHForRsync(ctx context.Context, sshCmd *exec.Cmd, command string) error { | ||
| sshRmCmd := exec.CommandContext(ctx, sshCmd.Path, append(sshCmd.Args, command)...) |
There was a problem hiding this comment.
| sshRmCmd := exec.CommandContext(ctx, sshCmd.Path, append(sshCmd.Args, command)...) | |
| // Skip Args[0] (program name) to avoid duplication | |
| sshRmCmd := exec.CommandContext(ctx, sshCmd.Path, append(sshCmd.Args[1:], command)...) |
…g the host files Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>
This PR adds a
--syncflag to thelimactl shellcommand that allows users to safely run AI commands inside VMs by syncing the host's working directory to the guest and optionally syncing changes back to the host after confirmation. Also added cleanup logic to remove the guest's synced workdir after user decision, ensuring no leftover files in the VM.How to test
When the program prompts the user to view the changes, a rsync is performed. This copies the guest synced directory to a temporary directory on the host. This allows diff to be used and the user to see a detailed list of changes.
Fixes #3711